Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Omit link to legacy list of React synthetic events in component documentation #519

Merged
merged 1 commit into from
Feb 28, 2024

Conversation

mbohal
Copy link
Contributor

@mbohal mbohal commented Jan 10, 2024

The page listing react synthetic events became legacy and was moved to https://legacy.reactjs.org/docs/events.html

I could not find a direct replacement, the closes thing I found is https://react.dev/reference/react-dom/components/common#common-props. This page however is not HTML element specific and thus I think it better to link do MDN even though there some differences (e.g. class vs className)

Also I have updated transferProps based on the new doc page.

@github-actions github-actions bot added the documentation Improvements or additions to documentation label Jan 10, 2024
@mbohal mbohal force-pushed the docs/fix-link-to-legacy-docs branch 2 times, most recently from bacd21a to 02c8215 Compare January 10, 2024 10:48
@adamkudrna
Copy link
Member

I could not find a direct replacement, the closes thing I found is https://react.dev/reference/react-dom/components/common#common-props. This page however is not HTML element specific and thus I think it better to link do MDN even though there some differences (e.g. class vs className)

It's not just the className. There are dozens of attributes (or props and events) that are React specific and cannot be found in HTML spec. I think this page ⬆️ is actually a very good replacement.

Also I'm unsure if we can omit such a basic thing as the onClick prop. So, I'd prefer a replacement rather than just deleting it without saying.

What does @bedrich-schindler think?

Also I have updated transferProps based on the new doc page.

I could not find this in the PR…

@mbohal
Copy link
Contributor Author

mbohal commented Jan 24, 2024

@adamkudrna I chnaged the wording to include a reference to both the MDN div attribute list and React common prop page as agreed on our video call.

Copy link
Member

@adamkudrna adamkudrna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect! This is much more helpful. 👏🏻

src/components/Alert/README.md Outdated Show resolved Hide resolved
@mbohal
Copy link
Contributor Author

mbohal commented Feb 15, 2024

In the end I arranged links to resources as a list because:

  1. It improves readability in both *.md and HTML versions
  2. It allows for uniform link labels without sacrificing readability

Copy link
Member

@adamkudrna adamkudrna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Beautiful job, thank you! 👏🏻

src/components/Tabs/README.md Outdated Show resolved Hide resolved
@mbohal mbohal force-pushed the docs/fix-link-to-legacy-docs branch from 12d4088 to 12171e1 Compare February 28, 2024 10:30
@mbohal mbohal merged commit 1800baa into master Feb 28, 2024
10 of 11 checks passed
@mbohal mbohal deleted the docs/fix-link-to-legacy-docs branch February 28, 2024 10:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants